Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

init.js revisited #538

Merged
merged 5 commits into from
Jul 5, 2023
Merged

init.js revisited #538

merged 5 commits into from
Jul 5, 2023

Conversation

danjoa
Copy link
Member

@danjoa danjoa commented Jul 3, 2023 •
edited
Loading

Follow up to #535
Ok, sorry, I tested with the existing bookstore sample, which has a similar reference to common data, and init.js was called after importing conflicting .csv files. But in this tests it is the other way around ...

I'd propose to change the implementation to this one to demonstrate best practices

@danjoa danjoa enabled auto-merge (squash) July 3, 2023 12:13
@johannes-vogel
Copy link
Contributor

I guess this works now but still seems a bit like a workaround to me that the init.js defers the actual initialization to a later point in time.

I know why we do it but not really intuitive if I don't know the reuse cases of bookshop.

@danjoa
Copy link
Member Author

danjoa commented Jul 4, 2023

I guess this works now but still seems a bit like a workaround to me that the init.js defers the actual initialization to a later point in time.

Yes, fully agreed, that's why we still didn't document it and shouldn't promote it.
Maybe we would rather introduce something like cds.before('deploy') and cds.after('deploy')

@danjoa
Copy link
Member Author

danjoa commented Jul 4, 2023

Any idea why these Fiori v2 tests fail?
They work locally with main

@johannes-vogel
Copy link
Contributor

Any idea why these Fiori v2 tests fail? They work locally with main

could be a new proxy version

@danjoa
Copy link
Member Author

danjoa commented Jul 5, 2023

Any idea why these Fiori v2 tests fail? They work locally with main

could be a new proxy version

You are right, v2 proxy@1.11 seems to have changed former hard-coded /v2 → / redirections to hard-coded /odata/v2 → /odata/v4 now. After adjusting all paths in #543 it seems to work again. Please approve that PR, then we can also merge this one.

@danjoa danjoa disabled auto-merge July 5, 2023 09:10
@danjoa danjoa merged commit 11770f6 into main Jul 5, 2023
2 checks passed
@danjoa danjoa deleted the int.js-revisited branch July 5, 2023 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johannes-vogel johannes-vogel johannes-vogel approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@danjoa @johannes-vogel